Skip to content

Comments

Add inner: &mut W parameter to Adapt closures and revise DrawShared::image_* fns#627

Merged
dhardy merged 6 commits intomasterfrom
push-qoouvwvtlzyr
Jan 7, 2026
Merged

Add inner: &mut W parameter to Adapt closures and revise DrawShared::image_* fns#627
dhardy merged 6 commits intomasterfrom
push-qoouvwvtlzyr

Conversation

@dhardy
Copy link
Collaborator

@dhardy dhardy commented Jan 3, 2026

Draft because: I'm undecided whether this is a good design; the purpose of Adapt is to add user-controllable state.

@dhardy
Copy link
Collaborator Author

dhardy commented Jan 3, 2026

I think Sprite could now be used something like this:

use kas::cast::Conv;
use kas::draw::ImageFormat;
use kas::event::TimerHandle;
use kas::geom::Size;
use kas::layout::LogicalSize;
use kas::Widget;
use kas_image::Sprite;
use kas_widgets::AdaptWidget;
use std::time::Duration;

const TIMER: TimerHandle = TimerHandle::new(0, true);

#[derive(Debug, Default)]
struct Buffer {
    size: Size,
    buffer: Vec<u8>,
}

async fn draw(mut buf: Buffer) -> Buffer {
    let len = usize::conv(buf.size.0) * usize::conv(buf.size.1);
    buf.buffer.resize(len, 0);
    // TODO: draw to buffer
    buf
}

enum State {
    Ready(Buffer),
    Drawing,
}

/// Make a sprite which draws itself in a worker thread on update
fn make_sprite(size: LogicalSize) -> impl Widget<Data = ()> {
    Sprite::new()
        .with_logical_size(size)
        .with_state(State::Ready(Buffer::default))
        .on_update(|cx, sprite, state, _| {
            if matches!(state, State::Ready(_)) {
                // We only have a ConfigCx here; use a timer to get an EventCx:
                cx.request_timer(TIMER, Duration::ZERO)
            }
        })
        .on_timer(TIMER, |cx, sprite, state, _| {
            if let State::Ready(buf) = std::mem::replace(state, State::Drawing) {
                if let Some(size) = sprite.allocate(cx) {
                    buf.size = size;
                    let id = cx.id();
                    cx.send_spawn(id, || draw(buf))
                }
            }
        })
        .on_message(|cx, sprite, buf| {
            if let Some(handle) = sprite.handle() {
                let draw = cx.draw_shared();
                if let Some(alloc_size) = draw.image_size(handle) && alloc_size == buf.size {
                    cx.draw_shared().image_upload(handle, &buf.buffer, ImageFormat::Rgba8);
                }
            }
        })
}

(I wanted to add this as example text, but cannot because kas-image does not depend on kas-widgets, while kas_widgets::Adapt is required here.)

I may merge this (or similar). @Kwarrtz can you test, or maybe update examples/proxy using an iterated function system?

@Kwarrtz
Copy link
Contributor

Kwarrtz commented Jan 5, 2026

I think iterated function systems might be needlessly complex as an example, but I can try to come up with something to demonstrate using Sprite. Maybe just drawing a gradient with changeable colors?

I'm a bit confused about the purpose of allocate here though. It re-allocates if the size of the allocated buffer and the Sprite's rect no longer match, but other than a call to set_logical_size, what could cause that to happen?

@dhardy
Copy link
Collaborator Author

dhardy commented Jan 5, 2026

Kas widgets don't choose their own size; they "suggest" size requirements; as such the size may be changed e.g. by the user resizing the window (depending on whatever layout is used; usually fixed-size widgets do get their chosen size). Since this is "logical size", the size could also change if the scale factor changes, e.g. by dragging the window to another screen.

The state machine is to only draw on demand (i.e. on resize), and if not already drawing. In your case you may still need something like this to respond to resizes. Also, you might want to use a double buffer (one to draw into, another to read from) to avoid tearing and making Mutex unhappy (assuming you use Arc<Mutex> to share the buffers and not just pass them to/from the rendering thread as here).

@dhardy dhardy marked this pull request as ready for review January 5, 2026 09:36
@dhardy
Copy link
Collaborator Author

dhardy commented Jan 5, 2026

I'm happy to merge this, though not completely happy about allocate; it's a bit awkward to use. Hence why having an example would be nice.

@Kwarrtz
Copy link
Contributor

Kwarrtz commented Jan 5, 2026

Ah, I see, I'd actually somehow missed that Sprite supports non-fixed sizes. If one wanted to reallocated and redraw on resize to stay pixel perfect (which seems to me like the main use-case for allocate), am I correct to think that that would need to happen during a set_rect call?

Also, if you're not happy how allocate works, maybe it could just be removed? If the redraw computation isn't instantaneous and is being forked off, like in your example, then it seems like you might want to defer calling Sprite::set until the new buffer is ready and so handle the re-allocation manually anyway. (Again, unless I'm misunderstanding the use case you had in mind for it.)

@dhardy
Copy link
Collaborator Author

dhardy commented Jan 5, 2026

If one wanted to reallocated and redraw on resize to stay pixel perfect (which seems to me like the main use-case for allocate), am I correct to think that that would need to happen during a set_rect call?

Yes. So on_update would need to be replaced with on_set_rect which doesn't exist yet. Good catch.

Also, if you're not happy how allocate works, maybe it could just be removed?

The allocation needs to happen somewhere. But yes, Sprite::allocate is purely a convenience function which could be replaced by DrawShared::image_alloc and Sprite::set.

Personally I would choose to use a custom widget at this point, but it may be useful to provide enough tools to let users implement something like this without a custom widget.

@dhardy
Copy link
Collaborator Author

dhardy commented Jan 5, 2026

Without Sprite::allocate, the above needs adapting to something like this (or a simpler version if you don't care about

        .on_timer(TIMER, |cx, sprite, state, _| {
            if let State::Ready(buf) = std::mem::replace(state, State::Drawing) {
                let draw = cx.draw_shared();
                let size = sprite.rect().size;
                if let Some(handle) = sprite.handle() && draw.image_size(handle) == Some(size) {
                    return;
                }
                
                if let Ok(handle) = draw.image_alloc(size.cast()) && sprite.set(cx, handle) {
                    buf.size = size;
                    let id = cx.id();
                    cx.send_spawn(id, || draw(buf));
                }
            }
        })

You could simplify a bit, but should certainly not re-allocate each time this is called since there's no way to ensure on_timer is called only once.

@Kwarrtz
Copy link
Contributor

Kwarrtz commented Jan 5, 2026

Yes, my thought was exactly that if a custom widget was needed for the set_rect behavior anyway, at that point calling image_alloc and set manually hardly seems prohibitively verbose. But if an on_set_rect adapter was added too, then maybe keeping the convenience function makes more sense. I'm just not sure how many applications there would be for that beyond this.

In either case, I'll try to put together a concrete example tomorrow.

@Kwarrtz Kwarrtz mentioned this pull request Jan 6, 2026
@dhardy dhardy force-pushed the push-qoouvwvtlzyr branch from f4a0621 to c431a1b Compare January 7, 2026 12:25
@dhardy dhardy changed the title Add inner: &mut W parameter to Adapt closures Add inner: &mut W parameter to Adapt closures and revise DrawShared::image_* fns Jan 7, 2026
@dhardy
Copy link
Collaborator Author

dhardy commented Jan 7, 2026

This got several updates motivated by #631:

  • Remove fn Sprite::allocate
  • Add fn Sprite::image_size
  • Let image_alloc use type Size
  • Require passing size into image_upload
  • Let image_upload return a Result<ActionRedraw, UploadError> to both notify of failures and nudge user to queue a redraw

@dhardy dhardy force-pushed the push-qoouvwvtlzyr branch from c431a1b to 5cdb278 Compare January 7, 2026 12:40
@dhardy dhardy merged commit d765163 into master Jan 7, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants